Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix race on precommit when cluster is changed #554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix race on precommit when cluster is changed #554

wants to merge 1 commit into from

Conversation

antonio2368
Copy link
Contributor

On config changes, the precommit index will be updated instantly to index of config change.
After it, request_append_entries is called which instantly commits up to precommit index if a node was removed from 2-node cluster.
If logs were being applied at the same time it could lead to commit thread trying to commit logs that were still not precommitted.
I couldn't find another explanation for such scenario.
If my conclusions are wrong feel free to correct me. 🙏
I also couldn't find a better way to fix it quickly.

@greensky00
Copy link
Contributor

greensky00 commented Nov 24, 2024 •
edited
Loading

Hi @antonio2368

Can you please help me understand the problem? From what I read your description,

  1. From a 2-node cluster, one member was removed, now there is only one member.
  2. Let's say there is a config change -- adding a new member.
  3. It will directly call store_log_entry, which internally updates pre-commit index.
  4. And then it will call request_append_entries, which immediately commits the log, as there is only one member.
  5. Background commit thread tries to execute the commit.

And your problem is, during 5, the log is not there yet at the moment of commit? But it is supposed to be in log store as log_store_->flush() is called in store_log_entry.

Can you please elaborate more on it? I wonder if I'm understanding it correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@antonio2368 @greensky00